Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes in and not_in filter views for integer, bigInt, decimal and float fields #7930

Merged
merged 4 commits into from
Oct 26, 2022
Merged

Fixes in and not_in filter views for integer, bigInt, decimal and float fields #7930

merged 4 commits into from
Oct 26, 2022

Conversation

nya1
Copy link
Contributor

@nya1 nya1 commented Sep 16, 2022

Closes #7638

In integer and bigInt fields I have changed the TextInput type to text (instead of number), this allows to enter commas, the filtering of the value (only digits, commas and minus sign) is already performed by a regex on the onChange handler.
With this change the filtering works as expected.

There is a comment above the TextInput regarding the type change but I'm not sure this is an old comment (prior to the onChange regex) or if we want to have a more advanced input.

keystone/packages/core/src/fields/types/integer/views/index.tsx

Lines 213 to 216 in df88fe6

// this should not be type=number since it shoud allow commas so the one of/not one of
// filters work but really the whole filtering UI needs to be fixed and just removing type=number
// while doing nothing else would probably make it worse since anything would be allowed in the input
// so when a user applies the filter, the query would return an error

@changeset-bot

This comment was marked as resolved.

Sign in to view
@vercel
Copy link

vercel bot commented Sep 16, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview Oct 26, 2022 at 6:24AM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 16, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8541b89:

Sandbox Source
@keystone-6/sandbox Configuration

@vercel vercel bot temporarily deployed to Preview September 16, 2022 11:58 Inactive
@flexdinesh flexdinesh mentioned this pull request Oct 23, 2022
Closed
@dcousens dcousens force-pushed the fix/7638-filter-search-number-fields-comma branch from 8dbc659 to 9832a32 Compare October 26, 2022 06:22
@dcousens dcousens changed the title fix: change input type of "integer" and "bigInt" fields to string Fix filter input types for integer, bigInt, decimal and float fields in the UI Oct 26, 2022
@vercel vercel bot temporarily deployed to Preview October 26, 2022 06:24 Inactive
dcousens
dcousens reviewed Oct 26, 2022
View reviewed changes
packages/core/src/fields/types/bigInt/views/index.tsx
? valueWithoutWhitespace.split(',').map(x => BigInt(x))
: BigInt(valueWithoutWhitespace);
? valueWithoutWhitespace.split(',')
: valueWithoutWhitespace;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a JSON serialization error

@vercel vercel bot temporarily deployed to Preview October 26, 2022 06:29 Inactive
@dcousens dcousens enabled auto-merge (squash) October 26, 2022 06:30
@dcousens
Copy link
Member

Thanks @nya1 and @gautamsi - it is unfortunate that these fields are not locale friendly.
I have updated the pull request to try and increase the consistency of the filters across each of the numeric fields, but honestly these need work and attention. 💛

We're currently experimenting with Adobe Spectrum in Keystone's updated design system, and that, or something like it, should help us unify and smooth the user experience across many of these field views.

@dcousens dcousens merged commit 109db89 into keystonejs:main Oct 26, 2022
@keystonejs-release-bot keystonejs-release-bot mentioned this pull request Oct 26, 2022
Merged
@gautamsi
Copy link
Member

We're currently experimenting with Adobe Spectrum in Keystone's updated design system, and that, or something like it, should help us unify and smooth the user experience across many of these field views.

does this mean end of @keystone-ui or just use their design system with @keystone-ui for implementing the design? you mentioned locale, is this going to help improve locale in admin-ui and content? Also new design system should make it possible to use new Admin-ui over the headless api instead of tightly coupled in terms of generation and consumption of admin meta.

@jossmac
Copy link
Member

jossmac commented Oct 27, 2022

@gautamsi We're interested in react-aria specifically for accessibility. Since they also provide I18N solutions we'll be considering that too.

The intention is to build a component library that's informed by and in service of, but decoupled from, the admin UI. Ideally consumers will be able to pick up the same pieces to build custom interfaces that fit seamlessly within Keystone's overall aesthetic and behaviour.

@keystonejs-release-bot keystonejs-release-bot mentioned this pull request Nov 3, 2022
Merged
@dcousens dcousens changed the title Fix filter input types for integer, bigInt, decimal and float fields in the UI Fixes in and not_in filter views for integer, bigInt, decimal and float fields Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcousens dcousens dcousens left review comments

@emmatown emmatown emmatown approved these changes

Assignees

@emmatown emmatown

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

is one of filter not working properly for integer field
5 participants
@nya1 @dcousens @gautamsi @jossmac @emmatown